Skip to content

[wip] adds simulation factory to builder #63

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 5 commits into from
Closed

[wip] adds simulation factory to builder #63

wants to merge 5 commits into from

Conversation

dylanlott
Copy link
Contributor

@dylanlott dylanlott commented Mar 17, 2025 •
edited
Loading

[wip] Adds Simulator to Builder

This PR adds the SimulatorFactory to the Builder.

  • adds initial implementation of simulator task to the builder
  • adds trevm @ 0.20.x

Related

Closes ENG-790
Closes ENG-957

This was referenced Mar 17, 2025
Closed
Closed
@dylanlott Graphite App
Copy link
Contributor Author

dylanlott commented Mar 17, 2025 •
edited
Loading

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@dylanlott dylanlott mentioned this pull request Mar 17, 2025
Closed
@dylanlott dylanlott marked this pull request as ready for review March 17, 2025 01:19
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from 53af4b3 to 018c274 Compare March 17, 2025 01:24
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch 3 times, most recently from b6464cd to 13a6ddb Compare March 17, 2025 02:11
@dylanlott dylanlott self-assigned this Mar 17, 2025
@dylanlott dylanlott added the enhancement New feature or request label Mar 17, 2025 — with Graphite App
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from 018c274 to f7df40c Compare March 17, 2025 18:37
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch from 13a6ddb to 3e6203e Compare March 17, 2025 18:37
@dylanlott dylanlott mentioned this pull request Mar 17, 2025
Closed
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch 2 times, most recently from b13b765 to bdc43a8 Compare March 17, 2025 18:48
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch from 3e6203e to 33f0c96 Compare March 17, 2025 18:48
@dylanlott dylanlott marked this pull request as draft March 17, 2025 20:48
@dylanlott dylanlott changed the title adds simulation factory to builder [wip] adds simulation factory to builder Mar 17, 2025
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from bdc43a8 to 0e77b32 Compare March 18, 2025 16:31
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch from 33f0c96 to 0f17597 Compare March 18, 2025 16:31
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from 0e77b32 to a18b444 Compare March 19, 2025 15:59
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch from 0f17597 to c0b070b Compare March 19, 2025 15:59
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from a18b444 to a21ba47 Compare March 19, 2025 16:25
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch from c0b070b to 56d569f Compare March 19, 2025 16:26
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from a21ba47 to 67e5fa9 Compare March 19, 2025 20:09
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch from 56d569f to 519bad3 Compare March 19, 2025 20:09
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from 67e5fa9 to 67dac1c Compare March 27, 2025 20:46
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch 7 times, most recently from b5d24dc to fa51801 Compare April 1, 2025 15:16
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from d8681d0 to d56a4e7 Compare April 3, 2025 19:06
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch 2 times, most recently from b6cef11 to 1bdd293 Compare April 4, 2025 21:53
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from 0921bcb to f302a1c Compare April 8, 2025 15:26
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch from 1bdd293 to 92cc1d6 Compare April 8, 2025 15:26
@dylanlott dylanlott force-pushed the 03-16-chore_update_to_alloy_0.11 branch from f302a1c to e0c79f7 Compare April 8, 2025 20:30
@dylanlott dylanlott force-pushed the 03-16-adds_simulation_factory_to_builder branch from 92cc1d6 to 3fac60c Compare April 8, 2025 20:30
@dylanlott
seed database for unit tests
e984e63 
- adds a seed_database function for seeding the cache db during testing
- gets tests passing to validate sim logic
prestwich
prestwich reviewed Apr 9, 2025
View reviewed changes
src/tasks/block.rs
@@ -43,6 +43,11 @@ impl InProgressBlock {
self.transactions.is_empty()
}

/// Returns the current list of transactions included in this block
pub fn transactions(&self) -> Vec<TxEnvelope> {
self.transactions.clone()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't clone, return a &[...]

prestwich
prestwich reviewed Apr 9, 2025
View reviewed changes
src/tasks/simulator.rs

impl<Db, Insp> SimulatorFactory<Db, Insp>
where
Insp: Inspector<Ctx<CacheOnWrite<CacheOnWrite<Arc<ConcurrentState<Db>>>>>>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why 2 CacheOnWrite?

@prestwich prestwich mentioned this pull request Apr 9, 2025
Closed
anna-carroll
anna-carroll reviewed Apr 10, 2025
View reviewed changes
src/tasks/simulator.rs
/// Scores are assigned by the evaluation function, and are Ord
/// or PartialOrd to allow for sorting.
#[derive(Debug, Clone)]
pub struct Best<T, S: PartialOrd + Ord = U256> {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is Best the clearest naming for this? what about SimulationResult? SimulationOutput?

src/tasks/simulator.rs
})
}

/// Simulates an inbound tx and applies its state if it's successfully simualted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: typo: simulated

src/tasks/simulator.rs
let best = Best { tx: Arc::new(tx), result, score };

// Flatten to save the result to the parent and return it
let db = db.flatten();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this might be a dumb question, but why are Best and SimResult two different types? could these just be one combined type?

src/tasks/simulator.rs

impl Cfg for PecorinoCfg {
fn fill_cfg_env(&self, cfg_env: &mut CfgEnv) {
cfg_env.chain_id = 17003;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this isn't the Pecorino chain id, is it? I believe it's 14174

also, what's this function intended to be? should this info be loaded from an environment config or something?

src/tasks/simulator.rs
}

// return the target account balance
let target_addr = address!("0x0000000000000000000000000000000000000000");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add TODO comment to replace this dummy value?

tests/simulator_test.rs
// Returns a new signed test transaction with default values
fn new_test_tx(wallet: &PrivateKeySigner, nonce: u64) -> eyre::Result<TxEnvelope> {
let tx = TxEip1559 {
chain_id: 17003,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chain ID doesn't match anvil chain ID setup above. maybe make a constant to use in the test so there isn't discrepancy?

tests/simulator_test.rs
let best = handle.await.unwrap();

// Assert on the block
assert_eq!(best.len(), 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why do we expect the block to contain one transaction, if we sent two valid transactions to the evaluator? if the second transaction should fail for some reason, can we leave a comment indicating why?

tests/simulator_test.rs
}

/// An example of a simple evaluator function for use in testing
fn test_evaluator(state: &ResultAndState) -> U256 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is duplicated from simulator.rs, should we import it to the test, or else only have it here if it's not meant for general consumption?

@dylanlott Graphite App
Copy link
Contributor Author

Closing this PR stack because we're moving simulation code into signet-sim crate in the signet-sdk for better encapsulation.

@dylanlott dylanlott closed this Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@prestwich prestwich prestwich left review comments

@anna-carroll anna-carroll anna-carroll left review comments

@Evalir Evalir Awaiting requested review from Evalir Evalir is a code owner

Assignees

@dylanlott dylanlott

Labels
enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@dylanlott @prestwich @anna-carroll